This PR brings back the old behaviour of using different sessions for writing/deleting RTIFs

I think has continued to use the same session. Pretty sure that the difference is pre-AIP-44 we committed in between.

Here's a way to explore:

session.execute("create table parent(id int);");
session.commit()
session.execute("insert into parent (id) values (1),(2),(3);")
with create_session() as session2:
    print(session2.execute("select * from parent").all())
assert session2 is session

Why this matters?

I feel like it's better to commit explicitly so that we see it. Rather than having it done invisibly by the decorator.

This PR brings back the old behaviour of using different sessions for writing/deleting RTIFs

I think has continued to use the same session. Pretty sure that the difference is pre-AIP-44 we committed in between.

Here's a way to explore:

session.execute("create table parent(id int);");
session.commit()
session.execute("insert into parent (id) values (1),(2),(3);")
with create_session() as session2:
    print(session2.execute("select * from parent").all())
assert session2 is session

Yeah, Issuing a commit persists things into the DB, not that the session is different.

Why this matters?

I feel like it's better to commit explicitly so that we see it. Rather than having it done invisibly by the decorator.

The provide session is already doing that? Or you want it removed from the RTIF write method? I feel it's a better design to have the provide_session do it than

RTIF.write(session)
session.commit()
RTIF.delete_old_record(session)
session.commit()

The provide session is already doing that? Or you want it removed from the RTIF write method?

It won't do it if you provide a session. So we don't necessarily have to remove the decorator, though doesn't seem a bad idea if we don't need it.

I feel it's a better design to have the provide_session do it than

Well, reasonable minds can differ. But what motivates me is "prefer explicit over implicit".

Do it as you wish, but I think when we let provide_session do the commits implicitly, it's much less obvious what's going on (and i think that's part of how we got into trouble here).

Commiting explicitly makes it very clear. It also affords an opportunity to explain why we commit there.

Anyway, most of this I think will be changed as part of AIP-72, so this i think is mainly academic, about us sort of just figuring out what's the best thing to do in this kind of scenario.

Rather than two commits you can also do session.flush() which makes SQLA issue the commands to the DB such that a future select on the same session sees the same rows, but it keeps it in the same session, such that the total operation is atomic still

Rather than two commits you can also do session.flush() which makes SQLA issue the commands to the DB such that a future select on the same session sees the same rows, but it keeps it in the same session, such that the total operation is atomic still

If I read this correctly, we would still have the issue: https://docs.sqlalchemy.org/en/20/orm/session_basics.html#flushing. Does this mean there was a flush in the update statement when we called write before the delete statement?

We aren't using 2.0 style though, so I think we have to call flush ourselves in 1.4/classic style